Skip to content

Conversation

dongjoon-hyun
Copy link
Member

@dongjoon-hyun dongjoon-hyun commented Mar 28, 2025

What changes were proposed in this pull request?

This PR aims to add cache and describe related SQL test and answer files.

  • cache.sql
  • clear_cache.sql
  • uncache.sql
  • describe_function.sql
  • describe_query.sql

Why are the changes needed?

To have a test coverage.

Does this PR introduce any user-facing change?

No, this is a test only change.

How was this patch tested?

Pass the CIs.

Was this patch authored or co-authored using generative AI tooling?

No.

@dongjoon-hyun
Copy link
Member Author

Could you review this PR, @attilapiros ?

@attilapiros
Copy link

attilapiros commented Mar 29, 2025

Should this be executed by the SQLTests.swift?

As that one contains a preprocessor macro checking the os and I think the test executions are skipped because of it:


@dongjoon-hyun
Copy link
Member Author

Yes, the current Github Action CI configuration is unable to cover 'collect' and 'show'-based integration test cases due to the Swift Linux implementation difference, @attilapiros .

Those integration test cases should be tested on MacOS-based CI or local Mac for now.

@dongjoon-hyun
Copy link
Member Author

This Linux-related issue has been a known limitation of Arrow Swift implementation so far and I've been working on in seperately.

Copy link

@attilapiros attilapiros left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

On Mac the test is running successfully:

◇ Suite SQLTests started.
◇ Test runAll() started.
binary.sql
cache.sql
clear_cache.sql
date.sql
describe_function.sql
describe_query.sql
floating.sql
integral.sql
pipesyntax.sql
select.sql
string.sql
uncache.sql
✔ Test runAll() passed after 0.174 seconds.
✔ Suite SQLTests passed after 0.174 seconds.

@dongjoon-hyun
Copy link
Member Author

Thank you so much, @attilapiros . Merged to main.

@dongjoon-hyun dongjoon-hyun deleted the SPARK-51659 branch March 30, 2025 22:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants